use lower cases for various file viewers #24
Conversation
m-y-mo
requested review from
JarLob,
kevinbackhouse and
sylwia-budzynska
as code owners
There was a problem hiding this comment.
Pull request overview
This pull request attempts to normalize GitHub repository owner and repository names to lowercase across various file viewer and resource management modules. However, this change introduces critical bugs that will break the functionality of all affected functions.
- Adds
.lower()calls to owner and repo parameters in 13 functions across 3 files - Affects both GitHub API interactions and local file system operations
- Applies the same transformation pattern consistently across all modified functions
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 13 comments.
| File | Description |
|---|---|
src/seclab_taskflows/mcp_servers/gh_file_viewer.py |
Adds lowercase conversion for owner/repo in 6 GitHub API functions (fetch_file_from_gh, get_file_lines_from_gh, search_file_from_gh, search_files_from_gh, list_directory_from_gh, search_repo_from_gh) |
src/seclab_taskflows/mcp_servers/local_file_viewer.py |
Adds lowercase conversion for owner/repo in 5 local file operations (fetch_file_content, get_file_lines, list_files, list_files_non_recursive, search_repo) |
src/seclab_taskflows/mcp_servers/local_gh_resources.py |
Adds lowercase conversion for owner/repo in 2 repository management functions (fetch_repo_from_gh, clear_local_repo) |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| owner = owner.lower() | ||
| repo = repo.lower() |
There was a problem hiding this comment.
Converting owner and repo names to lowercase will break file path lookups for repositories that were downloaded with their original case. The file system paths are constructed using the original case, so lowercasing these parameters will cause path mismatches and file-not-found errors.
| owner = owner.lower() | ||
| repo = repo.lower() |
There was a problem hiding this comment.
Converting owner and repo names to lowercase will cause API failures for repositories with uppercase characters in their names. GitHub's API is case-sensitive and requires the exact case of the repository owner and name. For example, calling the API with "github/copilot" when the actual repository is "GitHub/Copilot" will result in a 404 error.
| owner = owner.lower() | |
| repo = repo.lower() |
| owner = owner.lower() | ||
| repo = repo.lower() |
There was a problem hiding this comment.
Converting owner and repo names to lowercase will cause API failures for repositories with uppercase characters in their names. GitHub's API is case-sensitive and requires the exact case of the repository owner and name. For example, calling the API with "github/copilot" when the actual repository is "GitHub/Copilot" will result in a 404 error.
| owner = owner.lower() | ||
| repo = repo.lower() |
There was a problem hiding this comment.
Converting owner and repo names to lowercase will cause API failures for repositories with uppercase characters in their names. GitHub's API is case-sensitive and requires the exact case of the repository owner and name. For example, calling the API with "github/copilot" when the actual repository is "GitHub/Copilot" will result in a 404 error.
| owner = owner.lower() | ||
| repo = repo.lower() |
There was a problem hiding this comment.
Converting owner and repo names to lowercase will cause API failures for repositories with uppercase characters in their names. GitHub's API is case-sensitive and requires the exact case of the repository owner and name. For example, calling the API with "github/copilot" when the actual repository is "GitHub/Copilot" will result in a 404 error.
| owner = owner.lower() | ||
| repo = repo.lower() |
There was a problem hiding this comment.
Converting owner and repo names to lowercase will cause API failures for repositories with uppercase characters in their names. GitHub's API is case-sensitive and requires the exact case of the repository owner and name. For example, calling the API with "github/copilot" when the actual repository is "GitHub/Copilot" will result in a 404 error.
| owner = owner.lower() | ||
| repo = repo.lower() |
There was a problem hiding this comment.
Converting owner and repo names to lowercase will break file path lookups for repositories that were downloaded with their original case. The file system paths are constructed using the original case, so lowercasing these parameters will cause path mismatches and file-not-found errors.
| owner = owner.lower() | ||
| repo = repo.lower() |
There was a problem hiding this comment.
Converting owner and repo names to lowercase will break file path lookups for repositories that were downloaded with their original case. The file system paths are constructed using the original case, so lowercasing these parameters will cause path mismatches and file-not-found errors.
| owner = owner.lower() | |
| repo = repo.lower() |
| owner = owner.lower() | ||
| repo = repo.lower() |
There was a problem hiding this comment.
Converting owner and repo names to lowercase will break file path lookups for repositories that were downloaded with their original case. The file system paths are constructed using the original case, so lowercasing these parameters will cause path mismatches and file-not-found errors.
| owner = owner.lower() | ||
| repo = repo.lower() |
There was a problem hiding this comment.
Converting owner and repo names to lowercase will break file path lookups for repositories that were downloaded with their original case. The file system paths are constructed using the original case, so lowercasing these parameters will cause path mismatches and file-not-found errors.
|
Interesting. Copilot seems convinced that repo names are case sensitive. In practice, that seems to be not true, but I can't find any documentation to confirm it. |
No description provided.